chore: Implement credential type hierarchy #3738

lantoli · 2025-10-01T15:21:07Z

Description

Implement credential type hierarchy.

Note: Review of this PR might be easier looking at the new files instead of the differences.

Refactor provider and client codebase to reduce duplication and inconsistencies.
Rename token environment variable to MONGODB_ATLAS_ACCESS_TOKEN.

Link to any related issue(s): CLOUDP-345764

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

…tests

This reverts commit 9d778df.

This reverts commit 0aa6126.

…in unit tests" This reverts commit 2420ece.

…credentials_hierarchy * CLOUDP-334161-service-accounts-dev: chore: Remove all attributes in assume_role except role_arn (#3745) chore: Fix SA dev branch merge (#3744) chore: Fix cleanup GHA after SA changes (#3742) chore: Fixes backport release issues (#3734) chore: Support generation of singleton resources with no delete operation (#3736) # Conflicts: # internal/config/client.go # internal/config/transport_test.go # internal/provider/provider_sdk2.go # internal/service/organization/resource_organization.go # internal/service/organization/resource_organization_test.go # internal/testutil/acc/factory.go

Copilot

Pull Request Overview

This PR implements credential type hierarchy to refactor provider and client codebase, reducing duplication and inconsistencies. It also renames the token environment variable from MONGODB_ATLAS_OAUTH_TOKEN to MONGODB_ATLAS_ACCESS_TOKEN.

Introduces new credential type hierarchy to reduce duplication and improve consistency
Renames environment variable from MONGODB_ATLAS_OAUTH_TOKEN to MONGODB_ATLAS_ACCESS_TOKEN
Refactors provider configuration logic to use the new credential system

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/config/credentials.go	New file implementing credential type hierarchy with authentication methods and validation
internal/config/credentials_test.go	Comprehensive test coverage for the new credential system
internal/config/client.go	Refactored client creation to use new credential system with simplified API
internal/provider/aws_credentials.go	Simplified AWS credential handling to return Credentials struct
internal/provider/provider.go	Streamlined provider configuration using new credential hierarchy
internal/provider/provider_sdk2.go	Simplified SDK v2 provider configuration with new credential system
internal/testutil/acc/pre_check.go	Updated environment variable name for access token testing
internal/testutil/acc/factory.go	Updated to use new client creation API
internal/service/organization/resource_organization.go	Updated to use new client creation pattern
internal/service/organization/resource_organization_test.go	Updated test to use new client creation API
internal/service/eventtrigger/*.go	Updated realm client access pattern
internal/config/service_account.go	Simplified token source creation with cleaner parameters
internal/config/transport_test.go	Updated test to use new client creation API

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

internal/config/credentials.go

internal/config/client.go

Co-authored-by: Copilot <[email protected]>

oarbusi · 2025-10-06T09:14:39Z

internal/config/credentials.go

+	return a.AssumeRoleARN != ""
+}
+
+type Vars struct {


nit: maybe rename this to a more meaningful name? providerVars maybe?

Vars can be created from provider or environment variables using:

func NewEnvVars() *Vars # Create Vars from environment variables getProviderVars # Create Vars from TFP provider getSDKv2ProviderVars # Create Vars from SDKv2 provider

as I see it all these are provider vars, but can be inputed as provider inputs or env vars, so an option could be to have NewProviderEnvVars, getProviderInputsVars and getSDKv2ProviderInputsVars and change Vars name to ProviderVars. Feel free to ignore if to convinced, this is a nit on naming 😅

i think i understand the confusion, you call "provider inputs" when i say "provider". You're right that at the end of the day everything is about the provider :-)

"provider" var means defined in the Terraform provider config, e.g.:

provider "mongodbatlas" { client_id = "my_id" client_secret = "my_pass" }

and Vars can be obtained from the provider config or environment variables, credentials decision order is done here:

if c := CoalesceCredentials(providerVars.GetCredentials(), envVars.GetCredentials()); c != nil { ...

AgustinBettati · 2025-10-06T11:05:55Z

internal/config/credentials.go

+	if awsVars := CoalesceAWSVars(providerVars.GetAWS(), envVars.GetAWS()); awsVars != nil {
+		awsCredentials, err := getAWSCredentials(awsVars)
+		if err != nil {
+			return nil, err
+		}
+		return awsCredentials, nil
+	}
+	if c := CoalesceCredentials(providerVars.GetCredentials(), envVars.GetCredentials()); c != nil {
+		return c, nil
+	}


very clear and easy to follow 💯

AgustinBettati · 2025-10-06T11:09:32Z

internal/provider/provider.go

-	if data.AwsSecretAccessKeyID.ValueString() == "" {
-		data.AwsSecretAccessKeyID = types.StringValue(MultiEnvDefaultFunc([]string{
-			"AWS_SECRET_ACCESS_KEY",
-			"TF_VAR_AWS_SECRET_ACCESS_KEY",
-		}, "").(string))
-	}


(mentioned in complementary doc) only risk I see is that before we would allow combination of provider config + env variables, now it has to be all env vars or all provider config. For PAKs dont think a customer would ever do the mix, for the case of aws secret manager might be riskier. Would be transparent with a changelog entry.

added here, WDYT? cc @oarbusi 25c351f

I think the changelog is clear now. I think being able to use a mix of env vars or provider inputs should not be supported, but good if it's clear in the changelog

AgustinBettati · 2025-10-06T11:12:42Z

internal/config/client.go

+	AtlasV220240805  *admin20240805.APIClient // used in advanced_cluster to avoid adopting 2024-10-23 release with ISS autoscaling
+	AtlasV220240530  *admin20240530.APIClient // used in advanced_cluster and cloud_backup_schedule for avoiding breaking changes (supporting deprecated replication_specs.id)
+	AtlasV220241113  *admin20241113.APIClient // used in teams and atlas_users to avoiding breaking changes
+	Realm            *RealmClient


nice clean up removing config completely

github-actions · 2025-10-06T12:32:03Z

APIx bot: a message has been sent to Docs Slack channel

kanchana-mongodb

LGTM % copy

.changelog/3738.txt

Co-authored-by: kanchana-mongodb <[email protected]>

small change to trigger tests

0658fe4

lantoli changed the title ~~chore: Implement credential type hierarchy~~ chore: Implement credential type hierarchy Oct 1, 2025

lantoli added 9 commits October 1, 2025 19:20

don't use migration tests

bcb6cdb

rename credentials files to aws_credentials

425a8b0

refactor NewClient and create Credentials

be7f9a6

remove ctx param from NewClient

af627fe

keep only role_arn in assume_role

1800c31

fix TestAccServiceAccount_basic

8f796cc

TEMPORARY delete credentials from unit tests init

ccfca8a

dummy credentials in mocked unit tests

9d778df

TestPlanChecksClusterTwoRepSpecsWithAutoScalingAndSpecs only in unit …

2420ece

…tests

lantoli force-pushed the CLOUDP-345764_credentials_hierarchy branch from 64e7977 to 2420ece Compare October 3, 2025 07:02

lantoli added 2 commits October 3, 2025 09:07

Revert "dummy credentials in mocked unit tests"

0aa6126

This reverts commit 9d778df.

rename token env var to MONGODB_ATLAS_ACCESS_TOKEN

7770785

oarbusi mentioned this pull request Oct 3, 2025

chore: Rename JWT token environment variable #3743

Closed

13 tasks

lantoli added 4 commits October 3, 2025 09:54

Reapply "dummy credentials in mocked unit tests"

b61df5b

This reverts commit 0aa6126.

Revert "TestPlanChecksClusterTwoRepSpecsWithAutoScalingAndSpecs only …

e5c73bf

…in unit tests" This reverts commit 2420ece.

revert changes to acc test runner config

e9c477e

revert init changes

eeb1d5f

lantoli force-pushed the CLOUDP-345764_credentials_hierarchy branch 2 times, most recently from 24a610e to eeb1d5f Compare October 3, 2025 11:17

lantoli added 9 commits October 3, 2025 13:30

revert dummy credentials in mocked tests

d060712

initial AWS code

178b4b5

use env vars

1257c5d

remove CredentialProvider

f63dcf3

refactor AWS and Provider credentials

d700653

gov

9193eb2

configClient

61b62c5

refactor Realm client

104fb25

lantoli added 8 commits October 5, 2025 07:53

use same credentials struct in AWS Secrets Manager

b7e555b

method pick logic

c818683

config.GetCredentials

2cd7c13

warnings

7c56a75

unit tests

35353d3

fix linter

2afb3a3

don't log warnings in SDKv2 provider

e37facc

remove todo comments

f837b8e

lantoli marked this pull request as ready for review October 6, 2025 07:36

lantoli requested a review from a team as a code owner October 6, 2025 07:36

Copilot AI review requested due to automatic review settings October 6, 2025 07:36

Copilot AI reviewed Oct 6, 2025

View reviewed changes

internal/config/credentials.go Outdated Show resolved Hide resolved

internal/config/client.go Outdated Show resolved Hide resolved

lantoli and others added 2 commits October 6, 2025 09:49

Update internal/config/credentials.go

9e2ce3a

Co-authored-by: Copilot <[email protected]>

Update internal/config/client.go

c1153b5

Co-authored-by: Copilot <[email protected]>

oarbusi reviewed Oct 6, 2025

View reviewed changes

lantoli added 2 commits October 6, 2025 12:18

no need to get the credentials

f4bb882

fix if assume_role is defined but role_arn is not

5b7ba2a

AgustinBettati approved these changes Oct 6, 2025

View reviewed changes

manupedrozo approved these changes Oct 6, 2025

View reviewed changes

lantoli added 2 commits October 6, 2025 13:53

remove unused ctx param

5f2c411

changelog

25c351f

lantoli requested a review from a team as a code owner October 6, 2025 12:16

lantoli marked this pull request as draft October 6, 2025 12:31

lantoli marked this pull request as ready for review October 6, 2025 12:31

kanchana-mongodb approved these changes Oct 6, 2025

View reviewed changes

.changelog/3738.txt Outdated Show resolved Hide resolved

oarbusi approved these changes Oct 6, 2025

View reviewed changes

Update .changelog/3738.txt

6df44c7

Co-authored-by: kanchana-mongodb <[email protected]>

lantoli merged commit 5d09ca0 into CLOUDP-334161-service-accounts-dev Oct 6, 2025
90 of 91 checks passed

lantoli deleted the CLOUDP-345764_credentials_hierarchy branch October 6, 2025 13:11

chore: Implement credential type hierarchy #3738

chore: Implement credential type hierarchy #3738

Uh oh!

Conversation

lantoli commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change:

Required Checklist:

Further comments

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lantoli Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 6, 2025

Uh oh!

kanchana-mongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lantoli commented Oct 1, 2025 •

edited

Loading

lantoli Oct 6, 2025 •

edited

Loading